fix(blob): normalize contentDisposition in copy() when addRandomSuffix is enabled#1045
fix(blob): normalize contentDisposition in copy() when addRandomSuffix is enabled#1045matingathani wants to merge 1 commit intovercel:mainfrom
Conversation
…x is enabled When copy() is called with addRandomSuffix: true, the Vercel Blob API returns a contentDisposition header containing the suffixed filename (e.g. 'attachment; filename="report-abc123.pdf"'). The SDK now normalizes this to always use the original toPathname filename by moving the shared normalizeContentDisposition helper to put-helpers.ts and applying it in copy.ts, matching the behaviour already enforced for put(). Adds two regression tests: one confirming normalization fires on suffix mismatch, one confirming no-op when pathname is unchanged.
🦋 Changeset detectedLatest commit: 38491bb The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
@matingathani is attempting to deploy a commit to the Curated Tests - Permanent E2E Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Pull request overview
Fixes an inconsistency in copy() where the API may return a suffixed filename in contentDisposition when addRandomSuffix: true, even though callers expect the original toPathname filename.
Changes:
- Added a shared
normalizeContentDisposition()utility to rewrite the returnedContent-Dispositionfilename back to the originally requested filename. - Applied normalization in
copy()responses. - Added node regression tests and a patch changeset entry.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
packages/blob/src/put-helpers.ts |
Introduces normalizeContentDisposition() helper for adjusting API-returned contentDisposition. |
packages/blob/src/copy.ts |
Normalizes contentDisposition using toPathname vs API-returned pathname. |
packages/blob/src/index.node.test.ts |
Adds regression tests for copy() contentDisposition normalization behavior. |
.changeset/fix-blob-copy-content-disposition.md |
Documents the patch release for the copy() behavior change. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * instead of the API-returned pathname (which may include a random suffix). | ||
| * This ensures `contentDisposition` always reflects the name the caller | ||
| * provided, regardless of `addRandomSuffix` being enabled. |
There was a problem hiding this comment.
The docstring claims this "ensures contentDisposition always reflects the name the caller provided", but the implementation only rewrites when the header contains an exact quoted match of the response filename (and otherwise returns the original string). Consider tightening the wording to reflect the actual conditional behavior, or expanding the normalization to cover all expected Content-Disposition formats if the stronger guarantee is required.
| * instead of the API-returned pathname (which may include a random suffix). | |
| * This ensures `contentDisposition` always reflects the name the caller | |
| * provided, regardless of `addRandomSuffix` being enabled. | |
| * instead of the API-returned pathname (which may include a random suffix), | |
| * when the header contains an exact quoted match for the response filename. | |
| * If no such quoted filename is present, the original contentDisposition | |
| * value is returned unchanged. |
| When `copy()` is called with `addRandomSuffix: true`, the Vercel Blob API | ||
| returns a `contentDisposition` header containing the suffixed filename | ||
| (e.g. `attachment; filename="report-abc123.pdf"`). The SDK now normalizes | ||
| this to always use the original `toPathname` filename, matching the | ||
| behaviour already enforced for `put()`. |
There was a problem hiding this comment.
This changeset says the copy() normalization "matches the behaviour already enforced for put()", but put() currently returns response.contentDisposition unchanged (see packages/blob/src/put.ts). Either update the changeset wording to avoid claiming parity with put(), or update put() to use the same normalization helper so the statement is accurate.
Summary
The
copy()function has the samecontentDispositioninconsistency asput()(issue #903): whenaddRandomSuffix: trueis set, the Vercel Blob API returns a suffixed filename in theContent-Dispositionheader (e.g.attachment; filename="report-abc123.pdf"), but the caller expects the originaltoPathnamefilename.This PR:
normalizeContentDispositionfromput.tsintoput-helpers.tsas a shared utilitycopy.tsto normalize the returnedcontentDispositionBefore
After
Related: #903 (same fix applied to
put()in #1041)